-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release v2.6.3 #6604
Release v2.6.3 #6604
Conversation
6e98ca0
to
a9d2479
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## support/2.6.x #6604 +/- ##
================================================
Coverage ? 77.80%
================================================
Files ? 562
Lines ? 41846
Branches ? 0
================================================
Hits ? 32554
Misses ? 9292
Partials ? 0 ☔ View full report in Codecov by Sentry. |
Don't think we want to wait for this right? It is not critical and it seems it mostly depends on upstream dependencies that we don't control. Except for |
a9d2479
to
48516db
Compare
I agree with you that it is most likely it is only upstream dependencies, even though I would like to see the test run through. But by pinning the requirements, we are not compatible with Python 3.13. I am not sure how to deal generally with such a situation. I would have upper bounded the version in this release and made a new release once we have fixed the dependencies. But if you don't like it, we can just merge it like this. I think it is more important to bring the bugfixes out rather than finding the perfect solution for this. I added the commit e9d94f1 that fixes the pre-commit error, but did not include it in the CHANGELOG since it is just devops stuff that should not be relevant for enduser. Please let me know if you disagree. Also I added manually the commit 7f8d2dd to downgrade the The Docker Images CI fails here, but we have fixed it on main @unkcpz do you think it is relevant for the release? Cherry-picking the right commits would be a bit more work. |
Generally we don't do anything, and neither do most packages in the Python ecosystem I believe. The main point is to add an explicit lower bound.
We could start with this practice. I guess what would make most sense is to then simply always have an upper limit and when we add support for Python 3.13, we simply update the limit to
Perfectly fine. If there are bigger changes in devops, I tend to still add them because they can be useful for developers to quickly track down when something was changed, but for minor things like this there is no point really.
Yes. I would even have incorporated that change simply directly in the cherrypicked commit because it is really part of it. But this is fine as well. No need to change
If it is a lot of work, fine to skip. But if it can be fixed by one more cherry-picked commit, it may be useful. |
For the `aiida-core-with-services` image where the services are part of the image, we cannot rely on the health check for the services provided by docker-build as is used for the `aiida-core-base` case. Instead, a simple sleep was added to the `aiida-prepare.sh` script that sets up the profile, to make sure the services are up before the profile is created. This solution is neither elegant nor robust. Here the sleep approach is replaced by `s6-notifyoncheck`. This hook allows blocking the startup from continuing until a script returns a 0 exit code. The script in question first calls `rabbitmq-diagnostics ping` to make sure the RabbitMQ server is even up, followed by a call to `rabbitmq-diagnostics check_running`. If the latter returns 0, it means RabbitMQ is up and running and the script returns 0 as well, which will trigger `s6-notifyoncheck` to continue with the rest of the startup. Note that `rabbitmq-diagnostics is_running` could not be used as that command sometimes returns 0 even if the service is not ready at all. Co-authored-by: Sebastiaan Huber <[email protected]> Cherry-pick: 9579378
In e952d77 a bug in `verdi plugin list` was fixed where the conditional to check whether the plugin was a process class would always raise an `AttributeError` if the plugin was not a `Process` or a proces function. As a result, the code would never get to the else-clause. The else-clause contained itself another bug, which was now revealed by the fixing of the bug in the conditional. The else-clause would call the `get_description` classmethod of the plugin, but no classes in AiiDA that are pluginnable even define such a class method. Probably, the original author confused it with the instance method `get_description` but the `verdi plugin list` command just deals with the class. The `get_description` call is replaced with just getting `__doc__` which returns the docstring of the class/function, or `None` if it is not defined. In the latter case, a default message is displayed saying that no description is available. Since the else-clause code was never reached before the recent fix and the `get_description` method was never supported officially by AiiDA's pluginnable interfaces, it is fine to just change this behavior. Cherry-pick: c3b10b7
…ects (aiidateam#6566) Apparently, somehow `mypy` is updated and it is complaining about an `ignore[assignment]` comment that previously was imposed by `mypy` itself. (in src/aiida/orm/utils/serialize.py::L51) This commit removed `ignore[assignment]`, and `ci-style / pre-commit (pull_request)` is passing now. Cherry-pick: 655da5a
The current implementation only issues a kill command for the parent process, but this can leave child processes orphaned. The child processes are now retrieved and added explicitly to the kill command. Cherry-pick: fddffca Edits: Downgrades scheduler usage to old API in fix aiidateam#6572 The fix in aiidateam#6572 was pushed after the `Scheduler` API has been refactored in PR aiidateam#6043. To not include this breaking change in a minor release we adapt the usage of `Scheduler` in the PR to the old API.
…version (aiidateam#6581) The grep command failed that extracts the python version from mamba list failed because an indentation was added. See the python string in the output of `mamba list`: ``` List of packages in environment: "/opt/conda" Name Version Build Channel ──────────────────────────────────────────────────── python 3.10.13 hd12c33a_1_cpython conda-forge ``` Cherry-pick: 332a4a9
…ds not starting with aiida (aiidateam#6573) With the version update of bake-action the `BAKE_METADATA` a field with warning information (buildx.build.warnings) was added that does not contain the key `image.name` so the json query failed. With this commit another json query was added that filters out every field name that does not start with "aiida", so the field with warning information is filtered out. Co-authored-by: Jusong Yu <[email protected]> Cherry-pick: e1467ed
…idateam#6580) Processes start to broadcast their event before they update their process status in the database. This can cause issues if the next process directly tries to access the last process state retrieving it from the database while it has not been updated in the database. Cherry-pick: 867353c
48516db
to
b78c21f
Compare
I squashed it wtih the cherry picked commit. I added the edits message after the Cherry-pick hash to make it easier identifiable that this is a changed commit.
I tried to include some devops fixes but it ends becoming adding more and more commits until the CI is happy. To Fix the current error in the CI I need to add c52ec67, that requires cba6e7c, that is maybe breaking change, and more commits. I sadly don't have time to track this down this week. So I would release without it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @agoscinski Let's forget about the docker workflow and release this. Do you still have the time to do that? Are you familiar with the next steps for the release?
@sphuber should be fine. It is on pypi https://pypi.org/project/aiida-core/ and GitHub https://github.com/aiidateam/aiida-core/releases/tag/v2.6.3 I sadly forgot to update the date in the CHANGELOG :/ should have marked it in a TODO list. I think of a way to catch this in future releases. For now I wrote it in6o the wiki in bold. I followed further the instructions on the wiki and made a merge commit, but it seems that we have stopped doing this recently looking at the past releases. I actually like a merge commit that marks the commits the commits that are included. The commit message of the merge commit and the release commit (one before the merge commit) could be improved for the next release, right now they are a bit repetitive. |
Fantastic, thanks a lot for the work!
No biggy. Since we are not merging back the support branch to Also, maybe we want to update the version on
This approach of a merge commit to mark the release only really works for the support branch though where we are creating a release branch with cherry-picked commits that is then merged. But if we will release directly from |
@agoscinski would be good to update the CHANGELOG on main, since that's where people will generally look (at least that's what I do anyway). |
Still waiting if we merge PR #6600 before.